feat: bump to markdownlint-cli (vscode compatible) - #3402

markdownlint-cli2 is better for vscode integration.

Also there are 1795 linter warnings that I will try to modify.

Hello @alemorvan and welcome back to the project! :-) For the most part, I see no problems with the bulk of these changes, though I do, personally, hate dealing with multiple files in a single PR, particularly when they involve more than one team's docs. That said, I'm going to ask @tcooper to review the changes to the Testing team's docs. The remaining docs I think I can review and merge without much comment.

Trevor, the changes to the Testing team documents that are included in this PR are mostly formatting. For instance, @alemorvan rightly removes the HTML header information and replaces it with the markdown equivalent (i.e., <h3> = ###). Take a look at the files involved here and let me know if you have issues with any of the changes.

@sspencerwire / @alemorvan

Were these changes deployed in a test instance and verified before submission of this PR? That doesn't appear to be a requirement in the checklist and/or in the contributor docs.

I believe it used to be the case that a PR would create a vercel deployment that could be used to verify the changes before merge but that doesn't appear to be happening anymore.

If I recall correctly the use of html instead of markdown markup in the content include files is, or at least was, required when originally written. The modifications in the content includes files need to be manually verified to not break the includes.

Please do not merge this PR as written.

I have confirmed that changes to the ./include/... files to swap existing HTML <h3>[content]</h3> tags for their Markdown equivalent (=== [content]) has more effects than simply making a markdownlint-cli warning go away.

The most obvious effect is that heading of the ./include/... files gets added to the page Table of Contents menu that is generated by mkdocs. For pages already with TOC the item gets added while for pages without TOC (for example the Testing Team Members page) a previously non-existent TOC is generated.

Figure 1: Testing Team Members page from <main> branch, no TOC.

Figure 2: Testing Team Members page from <linter> branch, TOC added.

While this may seem like an innocuous result one additional and potentially undesired effect is that web crawlers will index the added TOC links. Since the purpose of the ./include/... files is to add repetitive boilerplate content to multiple pages this may not be desired.

In my opinion the markdownlint config should be modified to not report the use of <h#> (and possibly others) inside files in the ./include/... hierarchy as an issue needing to be fixed. Page authors can use the syntax that meets their needs with either the HTML or Markdown markup.

I will take this particular issue to the Testing Team to obtain a consensus opinion and report back here.

Hello @tcooper, @sspencerwire

@steven, I’ll try to create PRs that only include changes limited to certain folders to make things easier for you.

@trevor, I’m testing the rendering locally; I don’t compile the entire documentation for this kind of change.

Sometimes, translation and written communication can be complicated. I’d like to remind you that this work took me time and that I did it on a volunteer basis.

Let’s get back to the point: code quality.
We have a tool for writing documentation in Markdown, and you’re telling me that it’s “normal” to include HTML tags within the pages. This goes against the process we established several years ago. Some of us have a pre-commit hook to run the linter, and given the increasing number of lint errors, other contributors should do the same.

If what’s bothering you is the generation of a table of contents, then perhaps a good solution would be to use the mkdocs features:

---
hide:
  - toc
---

rather than trying to work around it.

If any other side effects come to mind, please let me know. I'd be happy to work with you to find solutions.

In the meantime, I'm putting this project on hold.

@alemorvan @tcooper Thanks for your review of this. This also goes a long way to show why including a lot of files in a single PR can cause potential issues, particularly where we are integrating other team's documents and build styles into the content. Limiting the number of files associated with a PR helps to keep this from happening. Our ultimate goal is to have good documentation for all, and that sometimes means making some compromises. Again, I appreciate your review. @alemorevan would you like to keep this PR here, or do you want it closed? At minimum, it should be made a draft until such a time as it is ready for merging.

@alemorvan / @sspencerwire The suggestion to disable TOC generation on pages that currently do not have it is easy to implement, thanks for that.

We discussed the effect on our remaining pages that already have a TOC and are fine with the addition of Additional Information to the bottom of the TOC even if the information is redundantly repeated across many pages in our Team area.

We will submit change requests for any of our pages where the TOC is not desired in a separate branch/PR.

In summary, the Testing Team is fine with these changes being applied to the ./include/... hierarchy and we appreciate being part of the discussion.

The Testing Team has not reviewed any of the other content of this PR.

@tcooper
Includes are handled by a Jinja filter. You cannot change the heading level natively.

Normally, the first heading in all documents—even included ones—should be a level 1 heading. Then, when including a document, you should specify a shift of one, two, or three levels so that the included content fits seamlessly into the chapter structure.

Here, you’ve chosen an arbitrary heading level: a level 3 heading, when there’s a good chance the required level is a level 2 heading.

There are two options available to us, but they go beyond the scope of this PR:

• Create a Jinja filter that shifts the heading level.

For example:

{% filter shift_headings(2) %}
{% include “teams/testing/content_bottom.md” %}
{% endfilter %}

and develop a Jinja filter called “shift_headings”.

Or use an inclusion plugin that already supports shifting, such as this one:
https://github.com/mondeja/mkdocs-include-markdown-plugin

It's worth looking into this issue

@sspencerwire

I'm not going to add any content to this PR.
I'll wait until it's merged before continuing with this in-depth work.

And I promise, I'll work on a smaller set of files ^^.